Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Infer server url on server select screen #1357

Merged
merged 27 commits into from
Oct 31, 2023
Merged

Infer server url on server select screen #1357

merged 27 commits into from
Oct 31, 2023

Conversation

sevenrats
Copy link
Member

@sevenrats sevenrats commented Sep 7, 2023 •
edited by cewert
Loading

remake of #1280

Allow the user to enter much less information, since entering info on the roku suxxx.
Users can now enter just the host, proto://host, or host:port, and we will check http and https at 80, 8096, 443, and 8920 appropriately.

fixes #366
fixes #283

@sevenrats sevenrats requested a review from a team as a code owner September 7, 2023 00:49
@sevenrats sevenrats marked this pull request as draft September 7, 2023 00:50
@sevenrats sevenrats marked this pull request as ready for review September 7, 2023 01:03
cewert
cewert requested changes Sep 7, 2023
View reviewed changes
Copy link
Member

@cewert cewert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass on the code. Haven't tested it yet

source/utils/misc.brs Outdated Show resolved Hide resolved
source/utils/misc.brs Outdated Show resolved Hide resolved
source/utils/misc.brs Outdated Show resolved Hide resolved
source/utils/misc.brs Outdated Show resolved Hide resolved
@sevenrats sevenrats requested a review from cewert September 7, 2023 02:11
@sevenrats
Copy link
Member Author

sevenrats commented Sep 9, 2023 •
edited
Loading

need to sanitize the path element in the regex so that it doesn't have trailing /
because if it did that would mess up the concatenation of /info/public

  • Done.

@sevenrats sevenrats marked this pull request as draft September 9, 2023 23:18
@cewert cewert removed their request for review September 10, 2023 15:38
@sevenrats sevenrats requested a review from cewert September 10, 2023 16:51
@sevenrats sevenrats marked this pull request as ready for review September 10, 2023 16:51
cewert
cewert requested changes Sep 10, 2023
View reviewed changes
source/utils/misc.brs Outdated Show resolved Hide resolved
cewert
cewert reviewed Sep 10, 2023
View reviewed changes
source/utils/misc.brs Outdated Show resolved Hide resolved
cewert
cewert reviewed Sep 10, 2023
View reviewed changes
source/utils/misc.brs Outdated Show resolved Hide resolved
@jellyfin-bot
Copy link
Contributor

This pull request has merge conflicts. Please resolve the conflicts so the PR can be reviewed. Thanks!

@jellyfin-bot
Copy link
Contributor

This pull request has been inactive for 21 days and will be automatically closed in 7 days if there is no further activity.

@jellyfin-bot jellyfin-bot added the stale This issue/PR has gone stale. label Oct 13, 2023
@cewert cewert removed the stale This issue/PR has gone stale. label Oct 14, 2023
@jellyfin-bot jellyfin-bot removed the merge-conflict This PR has a merge conflict label Oct 15, 2023
@1hitsong
Copy link
Member

Hard crash if user enters server value that begins with forward slashes: //128.1.8.8

@sevenrats
Copy link
Member Author

sevenrats commented Oct 15, 2023 •
edited
Loading

@1hitsong i'm not able to reproduce this can we see a crash log please

edit: I've reproduced it. The IP address has to be resolvable.

@cewert
Copy link
Member

cewert commented Oct 15, 2023

Pretty sure when he says hard crash he means that the roku device reboots which would mean no crash log.

@sevenrats
Copy link
Member Author

Pretty sure when he says hard crash he means that the roku device reboots which would mean no crash log.

the crash I've been able to reproduce is a softy. the spinner keeps going and the home button works. I can't get it to crash hard. What do you think should be done about it?

@1hitsong
Copy link
Member

Your latest changed fixed my crash.

@sevenrats
Copy link
Member Author

Your latest changed fixed my crash.

nice. good catch as always.

1hitsong
1hitsong suggested changes Oct 16, 2023
View reviewed changes
source/utils/misc.brs Outdated Show resolved Hide resolved
source/utils/misc.brs Outdated Show resolved Hide resolved
source/utils/misc.brs Outdated Show resolved Hide resolved
@jellyfin-bot jellyfin-bot added the merge-conflict This PR has a merge conflict label Oct 27, 2023
@jellyfin-bot
Copy link
Contributor

This pull request has merge conflicts. Please resolve the conflicts so the PR can be reviewed. Thanks!

@jellyfin-bot jellyfin-bot removed the merge-conflict This PR has a merge conflict label Oct 31, 2023
@sevenrats sevenrats requested a review from 1hitsong October 31, 2023 18:00
@1hitsong 1hitsong merged commit b8b6db9 into jellyfin:unstable Oct 31, 2023
9 checks passed
@cewert cewert added the feature A new feature that currently doesn't exist. label Nov 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cewert cewert cewert approved these changes

@1hitsong 1hitsong 1hitsong approved these changes

Assignees
No one assigned
Labels
feature A new feature that currently doesn't exist.
Projects
Ongoing development
Status: Done
next release
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve entering of server port number on setup screen [Feature] Attempt https
4 participants
@sevenrats @jellyfin-bot @1hitsong @cewert